Skip to content

[SYCL] Hide inline definitions of stdio functions #18174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 28, 2025

Conversation

hvdijk
Copy link
Contributor

@hvdijk hvdijk commented Apr 24, 2025

MSVC's inline definitions of stdio functions such as printf() are only meant for host code; device code cannot call them. Device code can, however, call ext::oneapi::experimental::printf() which depending on the target is implemented as a call to the global printf(). The availability of a definition of the global host printf() makes it more difficult to handle in device code, so hide it.

MSVC's inline definitions of stdio functions such as printf() are only
meant for host code; device code cannot call them. Device code can,
however, call ext::oneapi::experimental::printf() which depending on the
target is implemented as a call to the global printf(). The availability
of a definition of the global host printf() makes it more difficult to
handle in device code, so hide it.
@hvdijk hvdijk requested a review from a team as a code owner April 24, 2025 10:22
@hvdijk hvdijk temporarily deployed to WindowsCILock April 24, 2025 10:22 — with GitHub Actions Inactive
@hvdijk
Copy link
Contributor Author

hvdijk commented Apr 24, 2025

This fixes at least one SYCL E2E test (sycl/test-e2e/DeviceLib/built-ins/printf.cpp) when running on NativeCPU and presumably makes things easier for other SYCL targets as well.

Copy link
Contributor

@Fznamznon Fznamznon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not aware of implications of always enabling this macro, @AaronBallman can you maybe weigh in?

@hvdijk
Copy link
Contributor Author

hvdijk commented Apr 24, 2025

I'm not aware of implications of always enabling this macro

For the avoidance of confusion, this is inside a if (LangOpts.SYCLIsDevice) { block, I am not suggesting always defining this macro.

@AaronBallman
Copy link
Contributor

I'm not aware of implications of always enabling this macro, @AaronBallman can you maybe weigh in?

_NO_CRT_STDIO_INLINE controls whether there will be inline definitions of CRT IO functions in <conio.h>. When it's defined, the declarations are provided but with no definition. So I think this change is fine because it's guarded by SyclIsDevice.

@hvdijk
Copy link
Contributor Author

hvdijk commented Apr 28, 2025

@intel/llvm-gatekeepers This can be merged, thanks.

@sommerlukas sommerlukas merged commit 1dee646 into intel:sycl Apr 28, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants